Skip to content

ppc64-QVM: zero-extend cached OP_LOAD1/LOAD2 instead of sign-extend - #412

Merged
ec- merged 3 commits into
ec-:mainfrom
runlevel5:ppc64-qvm-load-zero-extend
Jun 30, 2026
Merged

ppc64-QVM: zero-extend cached OP_LOAD1/LOAD2 instead of sign-extend#412
ec- merged 3 commits into
ec-:mainfrom
runlevel5:ppc64-qvm-load-zero-extend

Conversation

@runlevel5

@runlevel5 runlevel5 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Fixes #411

The register-caching fast path reduced an already-cached wider value to
a byte/halfword with EXTSB/EXTSH (sign extend). OP_LOAD1/OP_LOAD2 are
unsigned loads, so any cached byte >= 0x80 (or halfword >= 0x8000) came
back negative, diverging from the interpreter and the fresh-load path
(LBZ/LHZ). This corrupted byte-sized cgame values and made HUD/item
icons disappear under the ppc64le JIT (CPMA, some OpenArena mods).

Use CLRLDI to zero-extend, matching reduce_map_size() unsigned constant
folding and the aarch64 JIT UXTB/UXTH. The genuine OP_SEX8/OP_SEX16
sign-extends are unaffected.

Fixes ec-/Quake3e#411
@runlevel5

Copy link
Copy Markdown
Contributor Author

@anengineer1 please try my fix

@anengineer1

Copy link
Copy Markdown
image

Sorry, no luck so far it seems

@runlevel5

Copy link
Copy Markdown
Contributor Author

@anengineer1 did you build with release type? cmake -B build -DCMAKE_BUILD_TYPE=Release?

@anengineer1

Copy link
Copy Markdown

It works, sorry, I confused PRs and I ended up testing the NaN one

@runlevel5
runlevel5 marked this pull request as ready for review June 29, 2026 22:47
…load

The cached OP_LOAD1/LOAD2 fast path zero-extended the cached register in
place. If that register was still a live value elsewhere on the opStack
(e.g. a wider constant reused across stores of different sizes, or a
prior load result still pending), the in-place reduction corrupted that
value - the JIT returned a result differing from the interpreter by a
power of two.

Mirror the clone-on-alias guard already used by load_rx_opstack/finish_rx:
when search_opstack() shows the register is live elsewhere, zero-extend a
copy and leave the cached register untouched; otherwise reduce in place
as before. OP_LOAD4 is unchanged.

Found and verified with a JIT-vs-interpreter differential fuzzer:
reproduced at iteration 17966 before, 0 divergences over 5,000,000
iterations after.
OP_LEF/OP_GEF mapped to a single CR0 test (branch if !GT / !LT). PPC
FCMPU sets only CR0[SO] when an operand is NaN, leaving LT/GT/EQ clear,
so those conditions wrongly took the branch on unordered operands -
diverging from the interpreter and aarch64 (where NaN <= / >= is false).

Fold the unordered bit into the tested bit via cror before the branch
(GT |= SO for <=, LT |= SO for >=) so a NaN operand does not branch.
Ordered comparisons and the other four float compares are unaffected;
the integer LE/GE paths are untouched.

Verified with a differential harness (JIT vs interpreter): NaN <= / >= 1.0
returned 1 on the JIT before and 0 after, matching the interpreter.
@ec-
ec- merged commit b04f5f5 into ec-:main Jun 30, 2026
28 checks passed

@ec- ec- left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thank you!
I guess that other JITs needs similar fixes, on x86/armv7l there is not much registers so search_opstack() never triggered for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Icons missing when using PPC64LE JIT

3 participants